-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v9] refactor!: instance descriptors, dynamically map ThreeElements #2465
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ee6bf62:
|
8632cdf
to
dc4e3b1
Compare
d1c1fcc
to
8ffad25
Compare
1104c5b
to
cb1c6b9
Compare
@joshuaellis, I'm not very clear on some of the semantics of RTTR's API. |
@CodyJasonBennett it's largely modelled of |
To clarify, I mean to do Fiber traversal based on the literal JSX passed to I wanted to clarify the intent behind class components since it was not clear whether |
I agree, I don't think
Sounds good to me! |
@joshuaellis, @drcmda, @gsimone, @krispya, any objections or withstanding issues before merging? Changes are mostly internal, but I've documented the implications and added test coverage for all the moving parts. I would break this up into another PR, but I didn't want to wait on coverage. |
r3f refactored their internal typings of Instance during pmndrs/react-three-fiber#2465 - before: Instance represented a THREE-object with additional parameters: __r3f: LocalState, children, remove, add and raycast - now: Instance: LocalState with additional parameter: object, props and children - object: is the THREE-object with additional parameter __r3f ( = a circular reference back to this Instance) - props contains the props of the component, without children and ref - children contain all the children of the component (before children only contained the attached children)
Continues #2378, dropping architecture changes. Fixes #2250.
Instance Descriptors
This PR creates instance descriptors to separate instance representations from their underlying objects. With them separated, we can avoid mutating objects with flags like
dispose={null}
and efficiently swap objects at runtime (e.g. inswitchInstance
). This allows the internal lifecycle of instances to change without breaking user code -- we still create objects increateInstance
but could defer that tohandleContainerEffects
if needed. Instances are an implementation detail of the reconciler, with elements' refs pointed toinstance.object
.react-three-fiber/packages/fiber/src/core/renderer.ts
Lines 33 to 45 in ee6bf62
Notably,
objects
is renamedchildren
and is used for all instances whereas previously it only contained attached children. Furthermore,memoizedProps
is renamedprops
and indiscriminately contains elements' props.Dynamically mapped ThreeElements
Additionally, three.js types for
JSX.IntrinsicElements
are no longer hard-coded and dynamically mapped toThreeElements
. This can let us relax the three peer dep significantly and make proper use of semver with types with the most recent feature we use beingTHREE.ColorManagement
of r139 and ambient@types/webxr
of r141. Consequently, theReactThreeFiber
type namespace is removed with elements' props being defined withinThreeElements
.react-three-fiber/packages/fiber/src/three-types.ts
Lines 1 to 61 in ee6bf62
Unified ThreeElement interface
When using extend, the
Object3DNode
,BufferGeometryNode
,MaterialNode
,LightNode
, andNode
interfaces are unified to justThreeElement<typeof CustomElement>
.ThreeElement
does not care about the element type, but will map properties based on their signature which best represents the behavior ofapplyProps
.Stable container-effects
Moves
attach
and interactivity tohandleContainerEffects
which will fire when trees are linked and finalized. This ensures that effects are not duplicated due to suspense and complete outside of commit, so we're never late and present an incomplete tree. Detailed in #2483.Internal Stability
As a follow-up to #2491, I've added coverage for utils and the reconciler lifecycle. This is basically a rewrite of renderer tests to fully cover the basics and ensure they work in difficult situations we've designed around.